Skip to content

fix(payments): page the payout ledger reads and verify they are complete (#533) - #536

Merged
guillermoscript merged 1 commit into
masterfrom
fix/533-payouts-unpaginated-ledger
Jul 25, 2026
Merged

fix(payments): page the payout ledger reads and verify they are complete (#533)#536
guillermoscript merged 1 commit into
masterfrom
fix/533-payouts-unpaginated-ledger

Conversation

@guillermoscript

Copy link
Copy Markdown
Owner

Closes #533.

What was wrong

getPayoutsOwed() read four whole relations — tenants, revenue_splits, transactions, payouts — with no .range() and no .limit(), then handed the arrays to computeOwedBalances(), which is pure arithmetic over whatever it is given.

PostgREST caps every response at the project's configured API "Max rows" and reports the cap exactly the way it reports a complete result: a 200 with fewer rows in it. Nothing downstream can tell them apart, so past the cap the balance simply comes out wrong, in whichever direction the truncation landed:

  • transactions truncated → grossOwed too low → underpays the school
  • payouts truncated → alreadyPaid too low → overpays it

And it was never display-only. markPayoutPaid() re-calls getPayoutsOwed() for its 10% mismatch guard, so a truncated balance also decided whether an operator got warned about a wrong payout amount — the guard would have waved through the very typo it exists to catch.

The tenants sweep in app/api/cron/enforce-plan-limits/route.ts had the same shape with a different symptom: past the cap the daily reconcile silently stopped visiting the tail of the tenant list, quietly disabling the only organic-growth trigger for #494's access cutoff.

The configured cap. supabase/config.toml sets max_rows = 1000 for the local stack. On the cloud project pg_db_role_setting holds no pgrst.* override, so it runs on Supabase's hosted default — also 1000. Both are now written down in docs/PLATFORM_ADMIN.md, but the fix is deliberately built not to depend on either being right.

What changed

lib/supabase/fetch-all-rows.ts (new). Pages a query until the relation is exhausted, then asserts the rows it actually collected against PostgREST's own count: 'exact' and throws — naming the relation and both numbers — when it comes up short. Callers must supply { count: 'exact' } (there is nothing to verify against otherwise) and a stable .order() on a unique column (unordered .range() windows can overlap or skip rows, which would trade a truncation bug for a duplication bug).

Two decisions worth reviewing:

  • The assertion is deliberately asymmetric. Fewer rows than the count is an error; more is fine, because rows inserted while the sweep runs are benign. Treating growth as corruption would fail the platform payouts page whenever a sale lands mid-render.
  • The loop self-tunes and never reads the cap. PostgREST clamps ranged requests too, so asking for a window wider than the cap returns a short page that is not the end of the relation — reading it as "done" would have reproduced the original bug inside the fix. Instead the loop adopts whatever page size the server proved it can return and continues from there. A MAX_PAGES ceiling stops a pathological server from spinning it forever.

app/actions/platform/payouts.ts — all four reads paged, each ordered by its primary key. Promise.all across the four relations is kept, so only pages within one relation serialize.

app/api/cron/enforce-plan-limits/route.ts — tenant sweep paged; an incomplete read now returns a 500 instead of quietly reconciling a partial list.

docs/PLATFORM_ADMIN.md — the rule, the confirmed cap, and the note that SQL-side aggregation (get_platform_stats, get_platform_revenue) remains immune by construction and is the better option for a large relation.

Why paging and not a SECURITY DEFINER aggregate

The issue offered either. An RPC would cut the transfer, but it would fork the arithmetic: lib/payments/payouts-owed.ts carries ~100 lines of documented, test-covered rules — the per-transaction split snapshot with its pre-#496 fallback, per-currency grouping, the reporting-only clawback heuristic, #516's overpaid term. Restating those in SQL creates two implementations that must agree forever, and the failure mode of them drifting is the same class of quiet wrongness this issue is about. Paging closes the correctness hole with one source of truth. If the transfer volume ever becomes the problem, the RPC is still available — that would be a performance change, made deliberately, not a correctness one.

Testing

  • npm run test:unit354 passed / 30 files. 12 new cases for fetchAllRows (short page, empty relation, multi-page accumulation, exact-multiple boundary, server cap below the requested page size, unrecoverable shortfall throws, concurrent-insert growth does not, moving count ignored, page-2 error propagates with its offset, runaway ceiling, default page size). The 30 existing payouts-owed cases stay green — the arithmetic module is untouched.

  • npm run typecheck — clean.

  • npm run build — clean.

  • npx eslint on changed files — 0 errors, 1 pre-existing unused-var warning in payouts.ts:13, untouched by this PR.

  • Verified against real PostgREST, not a mock. Forced the local stack's cap to 5 rows (ALTER ROLE authenticator SET pgrst.db_max_rows = '5') and ran the real queries against a 20-row table:

    [1] bare .select() returned 5 rows, no error  <-- silent truncation
    [2] fetchAllRows returned 20 rows (20 unique) vs 20 expected
    [3] tenants: 7 rows, complete   revenue_splits: 1   transactions: 0   payouts: 0
    [4] shortfall surfaced: fetchAllRows(question_options): incomplete read — fetched 5 of 20 rows.
    

    Note line 2: the helper was still requesting 500-row pages against a cap of 5 and returned everything, unique and in order. The cap was reset afterwards (pg_db_role_setting verified empty) and the throwaway script is not part of this branch.

QA script

No UI changed, so there is nothing visual to compare — the platform payouts page renders the same numbers, it just can no longer render wrong ones.

  1. Sign in as owner@e2etest.com (super admin) and open /platform/payouts. Balances render as before; per-currency figures unchanged against the seed data.
  2. Record a manual payout for a tenant. The 10% mismatch warning still fires on a deliberately wrong amount and still stays quiet on a correct one.
  3. curl -H "Authorization: Bearer $CRON_SECRET" localhost:3000/api/cron/enforce-plan-limits → same tenant tallies as before the change.
  4. To see the guard itself, repeat the cap experiment above: with pgrst.db_max_rows forced below a table's row count, the page must either render complete numbers or fail loudly — never show a plausible low one.

Out of scope

Surveying for this turned up the identical unpaginated-sweep shape in six other places, all service-role, all iterating the full array: solana-reconcile, binance-personal-reconcile, expire-subscriptions, solana-pull, expire-platform-subscriptions (four separate sweeps in that one), and app/actions/platform/billing-health.ts. None is in this issue's scope and each has its own blast radius worth reasoning about separately, but they now have a helper to adopt — worth a follow-up issue. app/sitemap.xml/route.ts also asks for .limit(5000) against a 1000-row cap, which is a quieter version of the same mismatch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T6Vfwhyj4Sktx8Am54cCGz

…ete (#533)

`getPayoutsOwed()` read `tenants`, `revenue_splits`, `transactions` and
`payouts` with no `.range()` or `.limit()`. PostgREST caps every response at
the project's configured API "Max rows" (1000, locally and on the cloud
project) and reports the cap as an ordinary 200 with fewer rows, so
`computeOwedBalances()` — pure arithmetic over whatever array it is handed —
produced a confidently wrong balance rather than an error. A short
`transactions` read underpays the school; a short `payouts` read overpays it.

It was not display-only: `markPayoutPaid()` re-calls `getPayoutsOwed()` for
its 10% mismatch guard, so a truncated balance also decided whether an
operator was warned about a wrong payout amount.

Add `lib/supabase/fetch-all-rows.ts`: pages a query until the relation is
exhausted, then asserts the rows it collected against PostgREST's own exact
count and throws — naming the relation and both numbers — on a shortfall.
Fetching more than the first count is fine (rows inserted mid-sweep); only a
shortfall means data was dropped.

The helper does not depend on knowing the cap. PostgREST clamps ranged
requests too, so a window wider than the cap comes back short without being
the end of the relation — the loop adopts the size the server actually
returned and continues. Verified against the local stack with the cap forced
to 5: a bare `.select()` returned 5 of 20 rows with no error, while
`fetchAllRows` returned all 20, unique and in order, still asking for
500-row pages.

Paging rather than a SECURITY DEFINER aggregate keeps one source of truth:
`payouts-owed.ts` carries the documented, test-covered rules (per-transaction
split snapshot with its pre-#496 fallback, per-currency grouping, the
reporting-only clawback heuristic, #516's overpaid term), and restating those
in SQL would create two implementations that must agree forever.

Same treatment for the `tenants` sweep in `enforce-plan-limits`, where a cap
silently stopped the daily reconcile from visiting the tail of the tenant
list — the only organic-growth trigger for #494's access cutoff. An
incomplete read now fails the run with a 500 instead of under-reporting.

Documents the rule and the confirmed cap in `docs/PLATFORM_ADMIN.md`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T6Vfwhyj4Sktx8Am54cCGz
@guillermoscript guillermoscript added bug Something isn't working Sub-task payments Payment provider / billing related labels Jul 25, 2026
@guillermoscript guillermoscript self-assigned this Jul 25, 2026
@guillermoscript
guillermoscript merged commit 3318403 into master Jul 25, 2026
2 checks passed
@guillermoscript
guillermoscript deleted the fix/533-payouts-unpaginated-ledger branch July 25, 2026 18:26
@guillermoscript

Copy link
Copy Markdown
Owner Author

Shipped in #536 (squashed to 3318403).

The four ledger reads in getPayoutsOwed() and the tenants sweep in enforce-plan-limits now go through fetchAllRows() (lib/supabase/fetch-all-rows.ts), which pages until the relation is exhausted and then checks the rows it collected against PostgREST's own count: 'exact' — a shortfall throws, naming the relation and both numbers, instead of returning a plausible low balance.

Answering the four asks in order:

  1. Paged, not moved into SQL. A SECURITY DEFINER aggregate would have forked the arithmetic: payouts-owed.ts holds the documented, test-covered rules (per-transaction split snapshot with its pre-Revenue split isn't time-sliced — plan changes retroactively reprice historical payouts #496 fallback, per-currency grouping, the reporting-only clawback heuristic, Payout overpayment is unrecoverable and the mismatch guard is skipped on retry (#499 follow-up) #516's overpaid), and restating them in SQL leaves two implementations that must agree forever. The RPC remains available later as a performance change, made on purpose.
  2. Completeness is asserted, and deliberately asymmetrically — fewer rows than the count is an error, more is not, since rows inserted mid-sweep are benign and treating growth as corruption would make the payouts page fail whenever a sale lands mid-render.
  3. enforce-plan-limits got the same treatment; an incomplete tenant list now fails the run with a 500 rather than quietly reconciling a partial one.
  4. Cap confirmed and written down in docs/PLATFORM_ADMIN.md: supabase/config.toml sets max_rows = 1000 locally, and the cloud project carries no pgrst.db_max_rows role override, so it runs on Supabase's hosted default — also 1000.

One thing worth recording, because it nearly shipped as a bug inside the fix: PostgREST applies the cap to ranged requests too. Asking for .range(0, 999) against a cap of 500 returns 500 rows, and a loop that reads "short page" as "end of relation" reproduces exactly the truncation this issue is about. The helper instead adopts whatever page size the server proved it can return and continues, so it never depends on the cap's value. Verified against the local stack with the cap forced to 5: a bare .select() returned 5 of 20 rows with no error, while fetchAllRows returned all 20, unique and in order, still requesting 500-row pages.

Left open on purpose — the same unpaginated-sweep shape exists in solana-reconcile, binance-personal-reconcile, expire-subscriptions, solana-pull, expire-platform-subscriptions (four sweeps in that one file) and app/actions/platform/billing-health.ts, plus app/sitemap.xml/route.ts asking .limit(5000) against a 1000 cap. Each has its own blast radius and deserves its own reasoning, but they all now have a helper to adopt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working payments Payment provider / billing related Sub-task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getPayoutsOwed reads the ledger unpaginated — a PostgREST row cap silently misstates payout balances

1 participant